Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clarify comparison of literals in SPARQL test suite #59

Closed
wants to merge 1 commit into from

Conversation

rubensworks
Copy link
Member

Closes #58

@gkellogg gkellogg requested a review from afs May 3, 2020 13:45
Copy link
Member

@gkellogg gkellogg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, I’m not sure this is correct. Graph isomorphism checks for same term, not equivalent value.

Tests that check for equivalent value wouldn’t use isomorphism.

@afs
Copy link
Contributor

afs commented May 3, 2020

Yes, https://www.w3.org/TR/rdf-concepts/#section-graph-equality defines same-term isomorphism; only blank nodes are remapped. There is a level above basic RDF, called D-entailment, that considers "same value".

Tests should be marked if they are value-sensitive but usually is in the test as FILTERS -- = compared to sameTerm.

Test should be marked up if they are value sensitive but may be some got missed.

PS as changes get made, can we keep text to a length that's reviewable in github please?

@gkellogg gkellogg self-requested a review May 3, 2020 19:47
Copy link
Member

@gkellogg gkellogg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually think that no change is necessary. As @afs says, if there are specific tests should be marked if they use value comparison, but this doesn't typically happen with CONSTRUCT, which is where graph isomorphism comes in, but with SELECT or ASK where the FILTER determines value equality. For example with expr-equals/query-eq2-1.rq.

PREFIX  xsd: <http://www.w3.org/2001/XMLSchema#>
PREFIX  : <http://example.org/things#>
SELECT  ?v1 ?v2
WHERE
    { ?x1 :p ?v1 .
      ?x2 :p ?v2 . 
      FILTER ( ?v1 = ?v2 ) .
    }

My feeling is that we should close the PR with no merge as being unnecessary. SPARQL 1.2 should be updated based on RDF 1.1 principles, and add any necessary clarification.

@gkellogg
Copy link
Member

gkellogg commented May 3, 2020

(In spite of saying I approved, my comment was that this should be closed without change).

@ericprud
Copy link
Member

ericprud commented May 4, 2020

I like the implementation advice in

Note that testing whether two result sets are isomorphic is simpler than full graph isomorphism. Iterating over rows in one set, finding a match with the other set, removing this pair, then making sure all rows are accounted for, achieves the same effect.

As a pedant, I think that approach of testing graph isomorphism works for comparing result set with rows of (?s, ?p, ?o), i.e. graphs. I propose changing that first sentence to

Result set isomorphism can be tested with this algorithm:

@rubensworks
Copy link
Member Author

The goal of this PR was to document the suggestion from @cygri and @afs:

So, as Andy said, the test suite documentation should state that literals are compared by value. Canonicalising actual and expected value before comparing them in the test runner is one way to achieve comparison by value.

The consensus from the comments above seem to be that instead of comparing literals in all tests like this, only tests that require this comparison should be marked with a flag.

I'm happy with either approach, so I'm fine with closing this PR (and creating a new one) if everyone agrees.

@gkellogg gkellogg closed this May 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

How to format decimals?
4 participants